Skip to content

fix: data plane request validation - #201

Open
LeeroyHannigan wants to merge 11 commits into
mainfrom
fix/data-plane-request-validation
Open

fix: data plane request validation#201
LeeroyHannigan wants to merge 11 commits into
mainfrom
fix/data-plane-request-validation

Conversation

@LeeroyHannigan

@LeeroyHannigan LeeroyHannigan commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

What

Data-plane request-validation fixes to align error classes, messages, and validation ordering with DynamoDB:

  • Validate item size before the table-existence check in PutItem and BatchWriteItem, so a malformed request against a non-existent table returns ValidationException instead of ResourceNotFoundException.
  • Return ValidationException (not SerializationException) for invalid ExpressionAttributeValues keys and values: validate all keys before parsing any value, and wrap value-validation errors as "<field> contains invalid value: <message> for key <key>".
  • Validate ExpressionAttributeValues number content (empty/non-numeric, >38 significant digits, overflow, underflow) and number-set duplicates, with the number error taking precedence over the more-than-one-datatype error. Report a non-true NULL, an unrecognized datatype, and multiple datatypes with the correct messages.
  • Report an empty binary value on a secondary-index key as "empty binary value" (was incorrectly "empty string value").
  • Route PutItem request deserialization through the shared error classifier so malformed-expression errors are consistently ValidationException.
  • Make GSI/LSI secondary-index pagination tests poll for eventual consistency (they assumed synchronous index propagation), and guard the zero-delay synchronous-index test to skip when the server is not booted in that mode rather than failing spuriously.
  • Add regression coverage for transactional updates that set empty string and empty binary values on non-key attributes.

Why

These behaviors diverged from DynamoDB (wrong error class, wrong message, or wrong validation ordering). Each fix was verified against the real DynamoDB service so the error type and message match.

Closes #

Testing done

  • cargo test --workspace (unit) — all pass.
  • Rust integration tests — 371 pass.
  • Python integration tests — 780 pass.
  • cargo fmt --all -- --check clean; cargo clippy --workspace --all-targets clean.
  • Every changed behavior was compared against the real DynamoDB service for exact error-class and message parity.

Checklist

  • I have read CONTRIBUTING.md
  • All tests pass (cargo test --workspace)
  • Code is formatted (cargo fmt --check)
  • Clippy is clean (cargo clippy -- -W clippy::pedantic)
  • I have added or updated tests for new functionality
  • I have updated documentation if behavior changed
  • Breaking changes are noted below (if any)
  • If this changes the wire protocol, Storage trait, auth model, on-disk format, or public CLI surface, an RFC has been accepted or is linked below. Otherwise, an ADR captures the decision (link below).

ADR / RFC: n/a — no change to wire protocol, Storage trait, auth model,
on-disk format, or public CLI surface. Error-class/message parity only.

Breaking changes

None. Only error responses for already-invalid requests change (from the wrong error class/message to the DynamoDB-correct one).


By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache License 2.0 and I agree to the Developer Certificate of
Origin (DCO). See CONTRIBUTING.md for details.

@LeeroyHannigan LeeroyHannigan changed the title Fix/data plane request validation fix: data plane request validation Jul 3, 2026
Comment thread tests/conftest.py Outdated
Comment thread tests/conftest.py Outdated
…tchWriteItem)

A genuinely oversized item sent to a non-existent table now returns
ValidationException ("Item size has exceeded the maximum allowed size")
instead of ResourceNotFoundException, matching Amazon DynamoDB (verified
against the service). The item-size limit is schema-independent, so it is
validated before the table is resolved; key-schema-dependent checks stay
after the existence check.

Also corrects the request-validation precedence tests: the previous
item-size case used a ~400,004-byte item, which is under the 409,600-byte
(400 KB) limit and so was never oversized -- it returned
ResourceNotFoundException only because the item was valid and the table
absent. The tests now use a genuinely oversized item and assert
ValidationException.

Signed-off-by: Lee Hannigan <lhnng@amazon.com>
… tests

Secondary-index (GSI) reads are eventually consistent — an item written to
the base table is not guaranteed to be visible through a GSI immediately, as
ExtendDB applies gsi_propagation_delay_ms like real DynamoDB. Several GSI
query/scan pagination and tiebreaker tests wrote items then paged the index
with no wait, asserting the full count, so they only passed when the delay
was zero and were racy otherwise.

Add a bounded wait_for_gsi_items() helper (15s) to conftest and use it in the
affected GSI query/scan pagination and tiebreaker tests so they poll until the
writes have propagated before asserting; a genuine drop still fails within the
bound rather than hanging.

Also guard the zero-delay synchronous-GSI test: gsi_propagation_delay_ms is
read by the data-plane write path at startup, so a runtime change only takes
effect if the server was booted with delay=0. The test now probes actual
behavior and skips (rather than falsely fails) when the running server is not
in synchronous mode.

Signed-off-by: Lee Hannigan <lhnng@amazon.com>
TransactWriteItems with an Update that sets non-key attributes to an empty
string or empty binary value must be accepted — empty values are valid on
non-key attributes, matching real DynamoDB — and must not fault. Existing
coverage exercised the transactional Put path and the empty-key rejection,
but not the transactional Update-to-empty-value path.

Add a Rust integration test asserting a transact Update that sets an empty
string and an empty binary on non-key attributes succeeds and reads the empty
values back.

Signed-off-by: Lee Hannigan <lhnng@amazon.com>
… message

An empty binary value on a secondary-index key attribute was rejected with a
message that hardcoded "empty string value" regardless of the attribute type,
so an empty binary index key was misreported as a string. Real DynamoDB
reports "empty binary value" for a binary key.

Make the secondary-index-key empty-value message type-aware (string vs binary)
for both the item and update-expression contexts. Add unit coverage for the
binary message in both contexts and a Rust integration test asserting an empty
binary GSI key is rejected with the type-correct message.

Signed-off-by: Lee Hannigan <lhnng@amazon.com>
Malformed values in ExpressionAttributeValues were mishandled: a non-true
NULL was rejected as a SerializationException instead of a ValidationException,
and empty/duplicate set values were rejected without the field/key context that
DynamoDB includes. A malformed value could also mask a malformed key in the
same map.

- Validate all ExpressionAttributeNames/Values keys before parsing any value,
  so a syntactically invalid key is reported ahead of value-content errors.
- Wrap semantic value-validation errors as
  "<field> contains invalid value: <message> for key <key>", matching DynamoDB.
- Treat a non-true NULL AttributeValue as a validation error.
- Include the "of type BS" qualifier in the binary-set duplicate message.

Adds unit coverage for the non-true NULL, empty/duplicate set, and
key-before-value precedence cases.

Signed-off-by: Lee Hannigan <lhnng@amazon.com>
PutItem deserialized its request with a private, stale copy of the error
classifier that omitted the field-prefixed "contains invalid key/value"
validation messages. A malformed ExpressionAttributeValues key or value on
PutItem therefore returned SerializationException where DynamoDB returns
ValidationException. Route PutItem through the shared deserialize_error
classifier already used by the other verbs so classification is consistent.

Signed-off-by: Lee Hannigan <lhnng@amazon.com>
An unrecognized AttributeValue datatype tag was rejected as a
SerializationException ("unknown type descriptor"), and a multiple-datatype
AttributeValue was rejected without the field/key context DynamoDB includes.

- Report an unrecognized datatype tag as "Supplied AttributeValue is empty,
  must contain exactly one of the supported datatypes" (no supported datatype
  is present), matching DynamoDB.
- Wrap unsupported-datatype and multiple-datatype value errors as
  "<field> contains invalid value: <message> for key <key>".

Adds unit coverage for both wrapped cases.

Signed-off-by: Lee Hannigan <lhnng@amazon.com>
Invalid numbers in ExpressionAttributeValues (and item attributes) were
accepted at deserialization and never rejected, so a malformed N/NS returned
the wrong error or none.

- Collect all AttributeValue map entries and validate N/NS number content
  before the "more than one datatype" check, so an invalid number is reported
  even alongside another datatype (matching DynamoDB precedence).
- Report a number-set duplicate as "Input collection contains duplicates".
- Extend the value-error wrapper and the shared deserialization-error
  classifier to cover the numeric-conversion, significant-digits, overflow and
  underflow messages, so they surface as ValidationException wrapped with the
  field name and key.

Adds unit coverage for the empty/non-numeric/overflow/underflow/duplicate and
multiple-datatype-precedence cases.

Signed-off-by: Lee Hannigan <lhnng@amazon.com>
- Drop the '(like real DynamoDB)' parenthetical: DynamoDB has no configurable
  GSI propagation delay; gsi_propagation_delay_ms is ExtendDB-specific.
- Reword 'can legitimately be short' -> 'can legitimately return fewer items
  than expected' for clarity.

Signed-off-by: Lee Hannigan <lhnng@amazon.com>
Signed-off-by: Lee Hannigan <lhnng@amazon.com>
@LeeroyHannigan
LeeroyHannigan force-pushed the fix/data-plane-request-validation branch from e7e69ff to 3aa3bce Compare July 17, 2026 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants